Skip to content

Update dependencies#783

Closed
getong wants to merge 5 commits intorust-bitcoin:masterfrom
getong:update-rand-0.9
Closed

Update dependencies#783
getong wants to merge 5 commits intorust-bitcoin:masterfrom
getong:update-rand-0.9

Conversation

@getong
Copy link
Copy Markdown

@getong getong commented Mar 2, 2025 •
edited by tcharding
Loading

Update dependencies:

  • rand to 0.9
  • bitcoin_hashes to 0.16
  • getrandom to 0.3

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 2, 2025

I guess we can move to rand 0.9 now that our MSRV is 1.63.0.

Can you update the lockfiles included in this crate? You can run ./contrib/update-lock-files.sh to do so.

@getong
Copy link
Copy Markdown
Author

getong commented Mar 2, 2025

I guess we can move to rand 0.9 now that our MSRV is 1.63.0.

Can you update the lockfiles included in this crate? You can run ./contrib/update-lock-files.sh to do so.

done

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 2, 2025

Thanks! Can you also update the getrandom dep to 0.3? As written this results in two versions of getrandom (and two versions of some of its wasm-related deps).

Finally, can I ask that you break this into three commits:

  • Update hashes
  • Update getrandom
  • Update rand

So that we can see the lockfile changes for each one?

@getong
Copy link
Copy Markdown
Author

getong commented Mar 3, 2025

Thanks! Can you also update the getrandom dep to 0.3? As written this results in two versions of getrandom (and two versions of some of its wasm-related deps).

Finally, can I ask that you break this into three commits:

  • Update hashes
  • Update getrandom
  • Update rand

So that we can see the lockfile changes for each one?

done

@getong getong changed the title update rand to 0.9, bitcoin_hashes to 0.16 update rand to 0.9, bitcoin_hashes to 0.16, getrandom to 0.3 Mar 3, 2025
@getong getong force-pushed the update-rand-0.9 branch from bef5cb3 to 2791338 Compare March 3, 2025 02:24
@getong getong mentioned this pull request Mar 3, 2025
Closed
8 tasks
@tcharding tcharding changed the title update rand to 0.9, bitcoin_hashes to 0.16, getrandom to 0.3 Update dependencies Mar 3, 2025
@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 3, 2025

I edited your PR description and title, hope you don't mind.

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 3, 2025

I can investigate your CI fail later on today if you like. I'm surprised to see it with these small changes.

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 3, 2025

Oh there are re-names that need doing in the docs e.g., thread_rng was renamed to rng.

@getong getong force-pushed the update-rand-0.9 branch from 1084f93 to 78e7479 Compare March 4, 2025 15:02
Kixunil
Kixunil reviewed Mar 4, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the last commit needs to be merged with the one updating rand.

Cargo.toml

hashes = { package = "bitcoin_hashes", version = "0.16", default-features = false, optional = true }
rand = { version = "0.9", default-features = false, optional = true }
rand = { version = "0.9", default-features = false, features = ["thread_rng"], optional = true }
Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't object to his but long term it'd be cool to propagate the feature.

@getong
Copy link
Copy Markdown
Author

getong commented Mar 4, 2025

getong#1

In this github ci , only two failed, but it does not matter with this repo code.

@Kixunil
Copy link
Copy Markdown
Collaborator

Kixunil commented Mar 4, 2025

@getong we have additional policy that each commit separately must pass tests. This is (currently) not checked by Github but @apoelstra has his own CI where he tests it.

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 5, 2025

The WASM error I think isn't related to this, but the other CI failure is because you're enabling the std feature of getrandom. I don't think we need this so it's ok to unconditionally disable it.

@getong getong force-pushed the update-rand-0.9 branch from 20a2351 to 78e7479 Compare March 6, 2025 02:32
@getong
Copy link
Copy Markdown
Author

getong commented Mar 6, 2025

The WASM error I think isn't related to this, but the other CI failure is because you're enabling the std feature of getrandom. I don't think we need this so it's ok to unconditionally disable it.

I try to add std feature to getrandom, but ci test still failed. So I delete std feature of getrandom.

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 7, 2025

Unfortunately you need to explicitly disable it with default-features = false.

@rustaceanrob rustaceanrob mentioned this pull request Aug 14, 2025
Closed
@tcharding tcharding mentioned this pull request Aug 19, 2025
Open
stevenroose
stevenroose reviewed Sep 5, 2025
View reviewed changes
Cargo.toml
secp256k1-sys = { version = "0.11.0", default-features = false, path = "./secp256k1-sys" }

hashes = { package = "bitcoin_hashes", version = "0.14", default-features = false, optional = true }
hashes = { package = "bitcoin_hashes", version = "0.16", default-features = false, optional = true }
Copy link
Copy Markdown
Contributor

@stevenroose stevenroose Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hashes has been removed IIRC.

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Sep 9, 2025

rand is at v0.9 already on master (and getrandom at v0.3). hashes was removed. This is not needed anymore.

@tcharding tcharding closed this Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kixunil Kixunil Kixunil left review comments

+1 more reviewer

@stevenroose stevenroose stevenroose left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@getong @apoelstra @tcharding @Kixunil @stevenroose